Skip to content

fix(review): suppress owned activity churn - #1007

Open
PollyBot13 wants to merge 1 commit into
openclaw:mainfrom
PollyBot13:fix/review-storm-self-activity
Open

fix(review): suppress owned activity churn#1007
PollyBot13 wants to merge 1 commit into
openclaw:mainfrom
PollyBot13:fix/review-storm-self-activity

Conversation

@PollyBot13

@PollyBot13 PollyBot13 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Scheduled hot intake can repeatedly run a full review on an unchanged issue or PR after ClawSweeper updates its own durable review comment or labels. GitHub may publish the item updated_at after the local sync timestamp, so bot-owned activity can be mistaken for fresh target activity.

Fixes #1006.

Why This Change Was Made

Production run https://github.com/openclaw/clawsweeper/actions/runs/30704608131 reviewed openclaw/clickclack#117 at unchanged head d26c11abba773132c51defb6e647db3a69610003. The review completed at 14:52:41Z, the durable comment synced at 14:53:28Z, and GitHub advanced the PR updated_at to 14:53:29Z. Scheduled hot intake then treated that server-side timestamp as uncaptured activity and ran another full review with zero cache hits.

The original version of this PR persisted the post-mutation timestamp and treated exact equality as ownership. Review correctly identified that as unsafe: an independent target-side update can share GitHub's second-level timestamp.

The revised design keeps the timestamp only as an observation and requires a complete structural ownership receipt before suppressing work:

  • scheduler and hot-intake paths fail open for timestamp-only or incomplete evidence;
  • structural-cache reuse requires matching source and non-automation timeline revisions;
  • PR receipts additionally bind the reviewed head and review-activity cursor;
  • apply/close paths require complete, non-truncated activity hydration;
  • same-second human activity invalidates the receipt, while same-second activity already included in the review remains covered;
  • post-proof covering-PR freshness is bound to a full structural snapshot digest rather than updated_at alone;
  • legacy records without the new receipt retain conservative behavior.

Fresh and cache-hit reports now persist review_timeline_revision, allowing exact same-second discrimination without weakening fail-open behavior.

User Impact

  • Stops ClawSweeper from amplifying its own label and durable-comment writes into repeated reviews.
  • Preserves immediate review eligibility for pushes, comments, reviews, state changes, and ambiguous activity.
  • Preserves daily and weekly coverage cadence.
  • Keeps existing records safe during rollout.

Evidence

Exact reviewed head: 84160ed710bb1dae8a14115a2babdeb823b81056
Base: 03f0344d21bf9ac724ff6d1d4a619308aa9fa6d6

  • pnpm run check passed: static checks, production and repair builds, dashboard build, lint, changed coverage, and the full 217-file test/coverage suite.
  • Full-suite coverage: 80.21% lines, 73.03% branches, and 86.81% functions.
  • Exact apply/close regression set passed 9/9, including truncated receipts, same-second new human activity, and already-reviewed same-second activity.
  • The focused scheduler, hot-intake, structural-cache, apply/close, semantic-cache, report, and covering-PR regressions pass.
  • git diff --check origin/main...HEAD passed.
  • Pre-commit review found the same-second reviewed-activity ambiguity; the patch added review_timeline_revision and both-direction regressions.
  • Fresh pre-commit and committed-range Codex reviews after that repair found no actionable regressions.

Current-head live GitHub proof

The disposable exact-head transcript is inspectable at PollyBot13#2 (comment).

Using the literal head and base above, the exact-head build captured live GraphQL structural records around two GitHub mutations:

  1. A controlled automation-owned label mutation advanced GitHub updatedAt while the filtered structural source revision remained identical. The scheduler admitted structural verification and the structural cache returned hit.
  2. A later target-side body edit changed the structural source revision. The scheduler again admitted verification and the same decision failed open with source_changed.

The proof PR is closed and disposable. It touched no production records, queues, review reports, workflows, or merge state.

Original production-ordering proof

The earlier disposable run remains inspectable at PollyBot13#1 (comment). It demonstrates the original delayed GitHub timestamp ordering on the first implementation head. The current-head proof above supersedes it for the revised structural-receipt behavior.

AI-assisted

Implemented and reviewed with Codex. Production GitHub state, repository history, both disposable proofs, the full local gate, and two independent committed-range reviews were used as evidence.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Aug 1, 2026
@clawsweeper

clawsweeper Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 3, 2026, 5:05 AM ET / 09:05 UTC.

ClawSweeper review

What this changes

The branch records source, timeline, pull-head, and review-activity receipts so ClawSweeper can avoid re-reviewing items changed only by its own GitHub comments or labels while preserving review for genuine activity.

Merge readiness

⚠️ Needs maintainer review before merge - 3 items remain

This PR remains necessary because it is the only open candidate fix for the concrete scheduler-churn report at #1006. Its structural-receipt approach is evidence-backed and fail-open, but GitHub reports a dirty merge and current main has since split the affected apply and review code, so it needs a careful rebase and fresh current-merge-head validation before landing.

Priority: P2
Reviewed head: 84160ed710bb1dae8a14115a2babdeb823b81056

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The receipt design has strong live behavior proof, but the dirty merge against a major current-main refactor prevents a merge-ready patch assessment.
Proof confidence 🦞 diamond lobster (5/6) ✨ media proof bonus Sufficient (linked_artifact): The PR body provides a current-head disposable live GitHub transcript that demonstrates an owned label update retaining a structural-cache hit and a subsequent body edit failing open; refreshed proof after rebase should redact any private operational data.
Patch quality 🦐 gold shrimp (3/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (linked_artifact): The PR body provides a current-head disposable live GitHub transcript that demonstrates an owned label update retaining a structural-cache hit and a subsequent body edit failing open; refreshed proof after rebase should redact any private operational data.
Evidence reviewed 5 items Open canonical bug: The PR body uses closing syntax for the open scheduler-churn report, whose observed production ordering matches this branch's owned-activity receipt design.
Current implementation boundary: Current main still treats post-review item timestamps and local synchronization clocks as scheduler activity inputs, which is the boundary the proposed receipt is intended to harden.
Merge conflict evidence: A three-way merge preview reports conflicts in the apply workflow, record metadata, report orchestration/rendering, review-command workflow, entrypoint, and apply-label-sync test; this is not a safe textual rebase.
Findings None None.
Security None None.

How this fits together

ClawSweeper schedules and caches reviews of GitHub issues and pull requests, then its apply lane publishes durable comments and labels or closes eligible items. This change carries post-publication activity evidence through scheduling, cache reuse, reports, and apply freshness checks so bot activity does not trigger another expensive review.

flowchart LR
  A[GitHub issue or pull request] --> B[Review scheduler]
  B --> C[Item context snapshot]
  C --> D[Structural cache]
  D --> E[Codex review]
  E --> F[Report receipt]
  F --> G[Apply comments and labels]
  G --> A
Loading

Before merge

  • Resolve merge risk (P1) - GitHub reports a dirty merge against current main; resolving only conflict markers could omit receipt checks now distributed across split apply and review modules, causing either renewed review churn or unsafe suppression of genuine activity.
  • Resolve merge risk (P1) - The branch changes automated scheduling, structural cache reuse, report metadata, and apply freshness checks together, so focused tests alone must be supplemented by proof on the rebased merge head.
  • Complete next step (P2) - The linked open issue remains valid, but the PR must be rebased and re-proven because its current merge result is dirty against the refactored automation paths.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Cross-cutting receipt change 20 files affected; production +268/-91, tests +360/-25 The receipt crosses scheduler, cache, report, and apply boundaries, so a clean rebase requires end-to-end validation rather than textual conflict resolution.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #1006
Summary: This PR is the open candidate fix for the scheduler-churn issue; earlier merged work addresses related but distinct duplicate-review paths.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Rebase the receipt design (recommended)
    Resolve the receipt implementation against the current apply and review module split, then prove scheduler, cache, and apply behavior on the resulting merge head.
  2. Pause the branch
    Pause or close this branch only if Scheduled hot intake re-reviews unchanged PRs after owned comment timestamps outrun sync metadata #1006 receives a narrower current-main replacement with equivalent fail-open guarantees.

Technical review

Best possible solution:

Rebase the structural-receipt design onto current main, preserve fail-open behavior for incomplete or changed snapshots across the split modules, and land it only after focused regressions plus a fresh current-head live proof confirm both owned activity reuse and real activity invalidation.

Do we have a high-confidence way to reproduce the issue?

Yes — the linked issue and PR body provide a concrete live GitHub ordering case, and the affected scheduler and structural-cache paths are identifiable on current main. This read-only review did not execute another live workflow.

Is this the best way to solve the issue?

Unclear until rebased — durable structural receipts are the appropriate fail-open direction, but this branch cannot yet demonstrate that the same guarantees survive the current apply and review architecture split.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 2d1a40bfa643.

Labels

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: Repeated reviews consume review capacity and create unwanted comment and label churn, but the reported behavior is not an emergency outage or data-loss condition.
  • merge-risk: 🚨 automation: The patch changes scheduling, cache reuse, and apply freshness guards that control automated GitHub review activity.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR body provides a current-head disposable live GitHub transcript that demonstrates an owned label update retaining a structural-cache hit and a subsequent body edit failing open; refreshed proof after rebase should redact any private operational data.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a current-head disposable live GitHub transcript that demonstrates an owned label update retaining a structural-cache hit and a subsequent body edit failing open; refreshed proof after rebase should redact any private operational data.

Evidence

What I checked:

  • Open canonical bug: The PR body uses closing syntax for the open scheduler-churn report, whose observed production ordering matches this branch's owned-activity receipt design. (84160ed710bb)
  • Current implementation boundary: Current main still treats post-review item timestamps and local synchronization clocks as scheduler activity inputs, which is the boundary the proposed receipt is intended to harden. (src/scheduler-policy.ts:84, 2d1a40bfa643)
  • Merge conflict evidence: A three-way merge preview reports conflicts in the apply workflow, record metadata, report orchestration/rendering, review-command workflow, entrypoint, and apply-label-sync test; this is not a safe textual rebase. (src/clawsweeper-apply-decision-workflow.ts:1, 2d1a40bfa643)
  • Current-main refactor provenance: Current main includes the review-rendering and automation split after this branch's base, explaining why receipt responsibilities must be reintegrated across the new module boundaries. (src/clawsweeper-review-command-workflow.ts:1, 2d1a40bfa643)
  • Scheduler history: The current scheduler hot-intake path was most recently changed by the merged exact-hot-intake suppression work, making that area relevant to rebase review. (src/scheduler-policy.ts:84, 0ced0b5e515b)

Likely related people:

  • Martin Cleary: Authored the merged exact hot-intake suppression change that most recently modified the scheduler policy now being extended. (role: recent scheduler contributor; confidence: high; commits: 0ced0b5e515b; files: src/scheduler-policy.ts)
  • Peter Steinberger: Authored the current-main review rendering and automation split that conflicts with the branch's apply and review workflow edits. (role: recent architecture contributor; confidence: high; commits: 2d1a40bfa643, 922ebc673e12; files: src/clawsweeper-apply-decision-workflow.ts, src/clawsweeper-review-command-workflow.ts, src/clawsweeper-review-planning.ts)
  • PollyBot13: Authored the proposed receipt change and the previously merged duplicate-check cache fix in the same review-churn area. (role: related cache contributor; confidence: medium; commits: 84160ed710bb, 12af8b7d0e0c; files: src/review-structural-cache.ts, src/scheduler-policy.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Rebase onto current main and retain all receipt checks across the split modules.
  • Run focused scheduler, structural-cache, and apply regressions on the merge head.
  • Update the PR body with current-head proof and request a fresh ClawSweeper review.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (12 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-02T08:55:15.585Z sha ca43c93 :: found issues before merge. :: [P1] Do not treat an equal timestamp as an ownership receipt
  • reviewed 2026-08-02T12:37:06.161Z sha 84160ed :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-02T12:49:05.637Z sha 84160ed :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T15:25:48.934Z sha 84160ed :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T17:36:00.671Z sha 84160ed :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T21:04:42.487Z sha 84160ed :: needs maintainer review before merge. :: none
  • reviewed 2026-08-03T01:08:36.048Z sha 84160ed :: needs maintainer review before merge. :: none
  • reviewed 2026-08-03T06:41:12.679Z sha 84160ed :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 1, 2026
@PollyBot13

PollyBot13 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Historical RCA: why the earlier fixes did not stop the review storm

Short version: no single recent PR introduced the whole failure. A latent timestamp-provenance ambiguity dates to the April scheduler work; later throughput changes amplified it from an occasional redundant review into a visible storm.

Timeline

  1. 21f9571 (25 April, ci: schedule reviews by activity cadence) made GitHub item updated_at part of review scheduling.
  2. 3ac50a8 (30 April, fix: harden clawsweeper scheduler fairness) added bot-owned synchronization timestamps. The safe boundary was effectively “at or before our locally recorded sync”; a later GitHub timestamp remained reviewable.
  3. #903 deliberately preserved that fail-open rule for activity 1–5 seconds after comment or label synchronization. That was the correct conservative choice without durable provenance: a nearby human update could not safely be distinguished from GitHub publishing ClawSweeper's own mutation late. The PR explicitly identified an exact-operation receipt as the proper future solution.
  4. #940, #955, #958, and #959 increased scheduled feed capacity, per-cycle candidate volume, and hot/normal cadence. They did not create the false-positive condition, but they gave it enough cadence and capacity to repeatedly select the same unchanged items.

Why #970 and #992 were insufficient

  • #970 fixed a separate duplicate path: equivalent CI check runs no longer create different cache identities for an unchanged head. That prevents duplicate CI data -> false cache miss, but it does not address ClawSweeper's own GitHub write -> false scheduler wake.
  • #992 correctly suppressed a fresh completed hot-intake review only when the persisted exact snapshot still matched the live PR. It intentionally failed open when activity timestamps differed. In the production failure, the local comment sync was recorded at 14:53:28Z, then GitHub authoritatively advanced the PR to 14:53:29Z; the snapshots therefore never matched, so the guard could not prove that the update was bot-owned.

Why the revised PR closes the gap

The original PR head used exact timestamp equality as ownership evidence. Review identified that as unsafe because independent target activity can share GitHub's second-level timestamp.

Current head 84160ed710bb1dae8a14115a2babdeb823b81056 keeps the authoritative timestamp only as an observation. Suppression now requires a complete structural receipt: source revision, non-automation timeline revision, and—for pull requests—the reviewed head and review-activity cursor. Incomplete or truncated evidence fails open. Exact regressions cover both directions of the same-second boundary: new human activity forces review, while activity already included in the reviewed timeline remains covered.

So the attribution is:

This also explains why the system appeared healthy before the July scaling work: the false-positive edge existed, but the older scheduler did not have the cadence and throughput to hammer it repeatedly.

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 1, 2026
@PollyBot13
PollyBot13 force-pushed the fix/review-storm-self-activity branch from ca43c93 to 84160ed Compare August 2, 2026 12:32
@PollyBot13

Copy link
Copy Markdown
Contributor Author

@codex review

@PollyBot13

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 2, 2026
@PollyBot13

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scheduled hot intake re-reviews unchanged PRs after owned comment timestamps outrun sync metadata

1 participant